Update for Pydantic v2 #1381
Update for Pydantic v2 #1381
Conversation
jwodder
added
minor
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1381 +/- ##
==========================================
+ Coverage 88.49% 88.57% +0.07%
==========================================
Files 77 77
Lines 10492 10493 +1
==========================================
+ Hits 9285 9294 +9
+ Misses 1207 1199 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
jwodder
force-pushed
the
gh-1352
branch
2 times, most recently
from
72ae5bf to
f256a55
Compare
|
@yarikoptic What was the timeline on getting this merged/released? |
|
Originally planned right after coming back from holidays, but that fell through. |
|
@yarikoptic Various tests are currently failing because the Archive doesn't yet accept schema version 0.6.5. |
|
@jwodder - could we TEMP test against version in
|
|
@yarikoptic Testing in dandi-cli requires a Docker image of the Archive PR. It would be easier to instead add a temporary commit in the Archive PR for running the CLI integration tests against this PR. |
|
|
||
| If ``path`` is already an absolute URL, it is returned unchanged. | ||
| """ | ||
| if path.lower().startswith(("http://", "https://")): |
There was a problem hiding this comment.
Function name and description suggests being applicable to URL of any transport, not just https so better be smth like
| if path.lower().startswith(("http://", "https://")): | |
| if re.match('[a-z]+://', path.lower()): |
to avoid surprises.
There was a problem hiding this comment.
but also how it is different from a standard urllib's join?
In [3]: urllib.parse.urljoin('http://a.c/', '/1/2/3')
Out[3]: 'http://a.c/1/2/3'
In [4]: urllib.parse.urljoin('http://a.c/', 's3://1/2/3')
Out[4]: 's3://1/2/3'
There was a problem hiding this comment.
-
The function is only meant for HTTP(S) URLs.
-
It differs from
urljoin()for URLs that have a path that doesn't end in/:>>> urljoin("https://example.com/foo/bar", "quux") 'https://example.com/foo/quux'joinurl()would instead givehttps://example.com/foo/bar/quux.
There was a problem hiding this comment.
- then better name it
httpurljoinor at least describe it in its description - also please add into description this difference in behavior as rationale
|
🚀 PR was released in |
Closes #1352.
Blocked by dandi/dandi-archive#1820.Do not merge until after dandi/dandi-schema#203 is merged & released. Once that is done, the "[TEMP] Use Pydantic 2.0 branch of dandischema" commit in this PR should be replaced with a commit that sets the[Done]dandischemaversion requirement to~= 0.9.0instead.